Skip to content

docs: ADR bundle (0001-0008) — engine and CoW architectural decisions - #12

Closed
brunota20 wants to merge 1 commit into
nullislabs:mainfrom
bleu:docs/adr-bundle
Closed

docs: ADR bundle (0001-0008) — engine and CoW architectural decisions#12
brunota20 wants to merge 1 commit into
nullislabs:mainfrom
bleu:docs/adr-bundle

Conversation

@brunota20

@brunota20 brunota20 commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Captures the eight architectural decisions Bleu has made (engine-side) or proposed (CoW host helpers, upstream policy, forward-looking factory pattern) across the M1 → M2 work. All eight carry status proposed pending upstream sign-off; landing them here makes the decisions reviewable independently from the implementing draft PRs (#8, #9, #10).

These ADRs went through internal Bleu review in bleu/nullis-shepherd#1 before opening here.

ADRs are ordered for reading (foundations → CoW infra → CoW helpers → cross-cutting), not chronologically.

Scope

ADR Title Implementing PR
0001 Operator engine.toml separate from module nexum.toml #8, #9
0002 Per-chain alloy provider transport selected by URL scheme #8, #9
0003 Per-module namespacing in local-store via length-prefixed key #8
0004 Patch cowprotocol to the head of upstream PR #5 #10
0005 cow-api host backend via cached OrderBookApi per chain #8
0006 TWAP and EthFlow as intent helpers in shepherd:cow@0.2.0 (design, not yet implemented)
0007 Push CoW Protocol logic to cow-rs first, adopt in engine second policy
0008 Dynamic address registration for log subscriptions (design, forward-looking, M3+)

Also adds data/ (engine state_dir default) and skills-lock.json (local skill lockfile) to .gitignore so the engine's runtime state does not pollute the working tree.

Format

ADRs follow the grill-with-docs format: short title, 1-2 paragraph Context, Decision, Considered options (rejected alternatives worth remembering), and Consequences. Frontmatter carries status: proposed and an implemented-in: <PR> pointer where applicable.

Points to weigh on review

  • ADR-0003 diverges from docs/04-state-store.md — the linked doc specifies one redb file per module; we propose a single shared file with a [len:u8][module][key] prefix. Rationale in the ADR. This is the one place Bleu deliberately deviates from the published design and needs explicit sign-off.
  • ADR-0006 is the M2 grant deliverable design — TWAP and EthFlow as thin host helpers backed by upstream cowprotocol primitives. EthFlow is positioned as the Rust externalization of the ethflow indexer currently inside cowprotocol/services/crates/autopilot/src/database/onchain_order_events/ethflow_events.rs.
  • ADR-0007 is the load-bearing policy — five protocol primitives (composable::poll_and_build_order with rich PollOutcome, eth_flow::decode_placement, OrderPostError rich variants + retry_hint, OrderBookApi::with_base_url, wasm32 feature-gate) land as additional commits on cowdao-grants/cow-rs PR #5 and are consumed via the patched dependency in ADR-0004.
  • ADR-0008 is forward-looking — does not affect M2 deliverables (TWAP and EthFlow use static subscriptions only). Lands in the 0.2 breaking-change window so adding factory support later does not require a major bump, but the design itself is the least firm of the eight. Internal review flagged the historical-backfill complexity; we already trimmed it. Happy to defer to 0.3 entirely if upstream prefers.

Next

Once feedback comes in, statuses flip to accepted (or superseded) in a follow-up commit. The most likely point of pushback is ADR-0003 (single redb file vs per-module file); the rest of the surface is either gap-filling or implementation choice over interfaces the upstream spec already defines.

@brunota20
brunota20 requested a review from mfw78 as a code owner June 3, 2026 22:31
Comment thread docs/adr/0001-engine-toml-separate-from-nexum-toml.md Outdated
Comment thread docs/adr/0002-provider-pool-transport-by-scheme.md
Comment thread docs/adr/0002-provider-pool-transport-by-scheme.md Outdated
Comment thread docs/adr/0003-local-store-namespacing.md
Comment thread docs/adr/0005-cow-api-via-cached-orderbookapi.md Outdated
Comment thread docs/adr/0006-cow-twap-ethflow-host-helpers.md
Comment thread docs/adr/0006-cow-twap-ethflow-host-helpers.md Outdated
Comment thread docs/adr/0007-upstream-protocol-logic-to-cow-rs.md Outdated
Comment thread docs/adr/0007-upstream-protocol-logic-to-cow-rs.md Outdated
Comment on lines +17 to +31
## Decision

Two pieces:

**1. Manifest schema gains `[[subscription.template]]`**, a topic-only log subscription whose address set is populated at runtime:

```toml
[[subscription.template]]
chain_id = 1
name = "uniswap_v3_pool"
event_topics = [
"0xc42079f94a6350d7e6235f29174924f928cc2ac818eb64fed8004e115fbcca67", # Swap
"0x7a53080ba414158be7ec69b987b5fb7d07dee101fe85488f0853ae16239d0bde", # Mint
]
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting. I'm not so sure on this. I'm pretty sure from my understanding that eth_getLogs is capable of being queried by topic without an address for the log to originate from. The question is whether or not this JSON-RPC method is supported by RPCs (i.e. is it a general case that public / free RPCs generally gate on requiring the address as well as the topics?).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. Combined with an internal complexity flag (the schema and host-function surface adds engine state that no M2 deliverable consumes), ADR-0008 has been deferred entirely (status: deferred, target 0.3). The reference design (Envio HyperIndex-style register-address + topic-only [[subscription.template]]) is preserved as historical context but does not land in 0.2. Topic-only eth_getLogs is explicitly listed as the simplest alternative for the eventual 0.3 conversation; the question of "do public RPCs gate on address" is the right one to validate at that point against a concrete consumer module.

Replaces the original docs/adr-bundle commit chain with the current accurate
version of the same file set. Each ADR + diagram now matches the design as
shipped through M5:

- ADR-0001 nexum.toml -> module.toml rename
- ADR-0003 32-byte hash namespacing in redb local-store
- ADR-0006 capability gating is link-time (boot validation against
  KNOWN_CAPABILITIES), not per-call dispatch
- diagrams.md ASCII hyphens per rubric; vapor features (cargo-nexum CLI,
  nexum-sdk separate crate, :8080/health endpoint, [module.resources]
  per-module caps) marked as future direction (0.3+ target), with pointers
  to ADR-0009 for the host-trait seam that supersedes the macro design.

Same file set as before (8 ADRs in docs/adr/, 9 diagrams in docs/diagrams/,
the original CI workflow + .gitignore + modules/example/Cargo.toml deltas).
This is a force-push that replaces the original PR #12 head; mfw78's prior
review threads will need to be re-anchored against the updated text.

AI Assistance: Claude Code (Opus 4.7) rebuilt the branch by copying file
content from chore/docs-reconciliation-m2-m5 (M5-current + reconciliation
applied) onto a fresh tree off the upstream main base. A human (Bruno) is
accountable for the result.
@brunota20

Copy link
Copy Markdown
Contributor Author

Heads up: I just force-pushed bleu:docs/adr-bundle to rebuild this PR with the reconciled, M5-current version of all the files originally proposed here. The file set is the same (8 ADRs in docs/adr/, 9 diagrams in docs/diagrams/, the CI workflow, .gitignore, modules/example/Cargo.toml), but the content has been updated to match what actually shipped through M2-M5:

  • ADR-0001: nexum.toml -> module.toml rename
  • ADR-0003: 32-byte hash namespacing in redb local-store
  • ADR-0006: capability gating is link-time (boot validation against KNOWN_CAPABILITIES), not per-call dispatch
  • docs/diagrams/diagrams.md: ASCII hyphens per rubric; vapor features (cargo-nexum CLI, separate nexum-sdk crate, :8080/health endpoint, per-module [module.resources] caps) marked as future direction (0.3+ target), with pointers to ADR-0009 for the host-trait seam that supersedes the macro design

Old head: e5579a3 (5 commits). New head: 439bd2f (1 squashed commit). Your prior review threads will need to be re-anchored against the updated text — apologies for the rebase noise, this was the cleanest way to align the PR with shipped reality before you do the final pass.

Surfaced via the 2026-06-23 conformance audit on the bleu fork (see bruno-brain/wiki/projects/shepherd-audits/m2-m5-conformance-2026-06-23.md for the trail).

mfw78 pushed a commit that referenced this pull request Jul 27, 2026
12 review threads addressed end-to-end. Net diff is -720 lines despite
adding ~200 lines of new helpers + tests, because the WitBindgenHost
adapter deduplication alone wipes ~400 lines.

Per-thread:

  #1 (balance-tracker architecture): refactored to match the M3
  host-trait+adapter split the other 4 modules use. Created
  `strategy.rs` with `on_block(&impl Host, ...)`, moved check_one /
  fetch_balance / parse_balance_hex / parse_settings into it,
  converted parse_config to use SDK config helpers + typed
  HostError instead of String. Added 3 MockHost-driven tests
  covering first-seen-above-threshold, below-threshold-persist,
  and error-does-not-abort-loop.

  #2 + #3 (WitBindgenHost dedup): new
  `shepherd_sdk::bind_host_via_wit_bindgen!()` declarative macro.
  Single source of truth in `crates/shepherd-sdk/src/wit_bindgen_macro.rs`;
  the 4 trait impls + convert_err / sdk_err_into_wit / convert_level
  collapse to one macro invocation per module. Migrated all 5
  modules (twap-monitor, ethflow-watcher, price-alert, stop-loss,
  balance-tracker). Each module's lib.rs lost ~80 lines.

  #4 (scale_decimal + config_get dup): new `shepherd_sdk::config`
  with `get_required`, `get_optional`, `scale_decimal`, and a
  typed `ConfigError` enum (host-neutral). price-alert + stop-loss
  consume the SDK helpers; their local duplicates were deleted.
  Module-level decimal-parsing tests removed (covered by 7 SDK
  tests + 4 proptest cases now).

  #5 (Chainlink dup): new `shepherd_sdk::chain::chainlink` with
  `read_latest_answer(host, chain_id, oracle, domain) -> Option<I256>`.
  Encapsulates the eth_call → parse → ABI decode flow + Warn
  logging. price-alert + stop-loss now call the helper; their
  local AggregatorV3 sol! definitions + read_oracle / on_block
  oracle plumbing was deleted. SDK ships with 3 StubHost tests
  covering happy path, host error, and garbage-hex.

  #6 (WIT world capability elision): added new "Capability
  enforcement vs. the WIT world" section to ADR-0009 documenting
  that price-alert + balance-tracker compile against the
  shepherd:cow/shepherd supertype but their manifests omit
  cow-api, and that boot success depends on wasm-tools' unused-
  import elision. Flagged as load-bearing; M5 macro hardening
  path documented.

  #7 (poll-time revert classification inert): filed COW-1082 for
  the host-side fix (forward structured eth_call error data into
  HostError.data; analogous to COW-1075 for orderbook).

  #8 (classify_api_error retry-default unbounded): filed COW-1083
  for the rate-limit / max-retry follow-up on the backoff: marker.

  #9 (RetryAction::Backoff dead variant): no code change; replied
  to thread clarifying it is reserved API surface waiting on a
  richer upstream retry_hint shape (open question for mfw78).

  #10 (no proptest anywhere): added `proptest` to shepherd-sdk
  dev-dependencies. New `crates/shepherd-sdk/src/proptests.rs`
  with 6 properties covering eth_call_params/parse_eth_call_result
  round-trip, parse_eth_call_result rejection on unquoted input,
  config::scale_decimal round-trip + sign-preservation, U256 LE
  byte round-trip, and no-panic guards for decode_revert_hex +
  gpv2_to_order_data marker dispatch.

  #11 (ethflow chain capability least-privilege): moved `chain`
  from required to optional in `modules/ethflow-watcher/module.toml`,
  mirroring the M2 mirror fix already applied.

  #12 (ADR-0009 test-count census): dropped the "145 host tests
  (twap 20, ethflow 12, ...)" breakdown; kept the qualitative
  claim. CI is now the authoritative count.

Drive-by: alloy-sol-types moved from regular to dev-dependencies in
price-alert and stop-loss now that the Chainlink ABI helper is
inside shepherd-sdk and the modules only use sol! in their test
helpers.

Validation:
- cargo test --workspace: every crate green; 5 modules + SDK + sdk-test + engine all pass. 8 host tests gained on balance-tracker; 6 proptest props gained on shepherd-sdk; 3 Chainlink helper tests gained.
- cargo clippy --workspace --all-targets -- -D warnings: clean.
- cargo fmt --check: clean.
- cargo build --target wasm32-wasip2 --release for all 5 modules: clean.
- Zero em-dashes in source code added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants